Skip to content

Conversation

@ion-elgreco
Copy link
Collaborator

Description

The description of the main changes of your pull request

Related Issue(s)

#2851

@ericvandever, this doesn't fix the coercion but allows you to still use large_dtypes=False to downcast so that you can still run most of your queries

@github-actions github-actions bot added the binding/python Issues for the Python package label Sep 7, 2024
@codecov
Copy link

codecov bot commented Sep 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.62%. Comparing base (ad35eda) to head (cbe2265).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2853      +/-   ##
==========================================
- Coverage   72.64%   72.62%   -0.02%     
==========================================
  Files         131      131              
  Lines       40015    40015              
  Branches    40015    40015              
==========================================
- Hits        29068    29062       -6     
  Misses       9087     9087              
- Partials     1860     1866       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rtyler
Copy link
Member

rtyler commented Sep 9, 2024

@ion-elgreco I'm sure you're getting busier as the autumn is starting, so i'll try to be quick:

  • Weren't we deprecating large_dtypes?
  • Since there's new logic here, would you have the time to write a test?
  • Does this work come back out at some point? Or does this branch need to exist as long as large_dtypes doe?

@rtyler rtyler changed the title chore: re-enable optional old casting behavior in merge fix: re-enable optional old casting behavior in merge Sep 9, 2024
@ion-elgreco
Copy link
Collaborator Author

@ion-elgreco I'm sure you're getting busier as the autumn is starting, so i'll try to be quick:

  • Weren't we deprecating large_dtypes?
  • Since there's new logic here, would you have the time to write a test?
  • Does this work come back out at some point? Or does this branch need to exist as long as large_dtypes doe?

We are, but some type coercion in Datafusion isn't working properly yet, particularly list <=> large list. So I only re add it temporarily.

The test will be short lived I guess, and the transformation code is well tested so should be fine.

I made an upstream issue for this type coercion problem

Last question I don't get? :p

rtyler
rtyler previously approved these changes Sep 12, 2024
@ion-elgreco ion-elgreco added this pull request to the merge queue Sep 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 12, 2024
@rtyler rtyler force-pushed the chore--allow-old-casting-behavior-until-deprecation branch from eb6d42a to cbe2265 Compare September 12, 2024 15:27
@rtyler rtyler enabled auto-merge September 12, 2024 15:27
@rtyler rtyler added this pull request to the merge queue Sep 12, 2024
Merged via the queue into delta-io:main with commit 47376a6 Sep 12, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binding/python Issues for the Python package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants